Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors middleware imports/configuration, strengthens server startup/shutdown logic, tightens type safety (especially in database helpers and HTTP tests), and updates tooling and documentation.
- Reorganized middleware imports and updated configurations (CORS, Morgan, Helmet, JSON error handler)
- Added graceful shutdown handling and improved dotenv/loading order in
src/index.ts - Refactored store CRUD functions with stricter types in
src/database, and added comprehensive type-safe assertions in HTTP tests - Expanded package scripts, added ESLint/Prettier configs, and corrected documentation in README
Reviewed Changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/midleware/morgan.ts | Removed unused StreamOptions import |
| src/midleware/json.ts | Adjusted reviver signature and reordered options |
| src/midleware/helmet.ts | Reordered CSP/HSTS/referrerPolicy options |
| src/midleware/errorHandler.ts | Marked unused parameters, added stack-trace fallback |
| src/midleware/cors.ts | Switched to nullish coalescing, trimmed headers/methods arrays |
| src/index.ts | Loaded dotenv earlier, added gracefulShutdown, minor logging |
| src/http_tests/stores.test.ts | Added type assertions and conditional guards on IDs |
| src/http_tests/authentication.test.ts | Introduced AuthResponse type and scoped Response imports |
| src/http_tests/app.test.ts | Introduced ResponseBody type and scoped Response imports |
| src/database/stores.ts | Reordered exports, stricter Store type, refactored CRUD logic |
| src/database/mongo-common.ts | Unified getDatabase/startDatabase, enforced MONGO_URL |
| src/app.ts | Reordered route/middleware imports |
| package.json | Added private flag, expanded scripts, import maps, lint/prettier |
| eslint.config.js | Added TS-aware ESLint config with perfectionist plugin |
| README.md | Updated env vars list, scripts, formatting |
| .prettierrc & .prettierignore | Added Prettier configuration and ignore file |
Comments suppressed due to low confidence (1)
src/http_tests/stores.test.ts:55
- [nitpick] Wrapping the update/delete assertions inside
if (storeId)can silently skip tests whenstoreIdis undefined. Consider assertingexpect(storeId).toBeDefined()before or failing fast.
if (storeId) {
| return { | ||
| _id: updated._id?.toString(), | ||
| name: updated.name, | ||
| _id: new ObjectId(updated._id), |
There was a problem hiding this comment.
Avoid wrapping an existing ObjectId in new ObjectId(...), which will generate a new ID. Use the returned updated._id directly or call updated._id.toString() if a string is required.
Suggested change
| _id: new ObjectId(updated._id), | |
| _id: updated._id, |
| name: store.name, | ||
| addedBy: store.addedBy, | ||
| })) as Store[]; | ||
| return (await database.collection(collectionName).findOne({ _id: insertedId })) as null | Store; |
There was a problem hiding this comment.
The returned document’s _id is an ObjectId. If your API or callers expect a string ID, convert it using inserted._id.toString() before returning.
Suggested change
| return (await database.collection(collectionName).findOne({ _id: insertedId })) as null | Store; | |
| const insertedStore = await database.collection(collectionName).findOne({ _id: insertedId }); | |
| if (!insertedStore) return null; | |
| return { | |
| ...insertedStore, | |
| _id: insertedId.toString(), | |
| } as Store; |
| const database = await getDatabase(); | ||
| const stores = await database.collection<Store>(collectionName).find({}).toArray(); | ||
| return stores.map(store => ({ | ||
| _id: store._id, |
There was a problem hiding this comment.
Map the ObjectId to a string (e.g. store._id.toString()) so that JSON serialization and clients receive a plain string identifier.
Suggested change
| _id: store._id, | |
| _id: store._id.toString(), |
Comment on lines
3
to
+9
| import compression from './midleware/compression'; | ||
| import cors from './midleware/cors'; | ||
| import errorHandler from './midleware/errorHandler'; | ||
| import helmet from './midleware/helmet'; | ||
| import json from './midleware/json'; | ||
| import cors from './midleware/cors'; | ||
| import morgan from './midleware/morgan'; | ||
|
|
||
| import notFoundRouter from './routes/notFoundRoute'; | ||
| import rateLimiter from './midleware/rateLimiter'; |
There was a problem hiding this comment.
[nitpick] The folder name midleware is misspelled; it should be middleware to match standard conventions and avoid confusion.
|
|
||
| server = app.listen(PORT, () => { | ||
| console.log(`Server running without DB on port ${PORT}`); | ||
| console.log(`Server running without DB on port ${PORT.toString()}`); |
There was a problem hiding this comment.
[nitpick] Calling .toString() inside a template literal is unnecessary—template interpolation will convert PORT to string automatically.
Suggested change
| console.log(`Server running without DB on port ${PORT.toString()}`); | |
| console.log(`Server running without DB on port ${PORT}`); |
| 4. ALLOWED_METHODS (optional) | ||
| 5. ALLOWED_HEADERS (optional) | ||
| 6. NODE_ENV=test --- When set to ***"test"***, a `mongodb-memory-server` test URI is used, and no `MONGO_URL` is required. This allows for out-of-the-box testing without a live database. | ||
| 6. NODE\*ENV=test --- When set to \*\*\*"test"\_\*\*, a `mongodb-memory-server` test URI is used, and no `MONGO_URL` is required. This allows for out-of-the-box testing without a live database. |
There was a problem hiding this comment.
This line contains malformed Markdown and an incorrect environment variable name (NODE*ENV). It should read NODE_ENV=test without backslashes and with proper emphasis syntax.
Suggested change
| 6. NODE\*ENV=test --- When set to \*\*\*"test"\_\*\*, a `mongodb-memory-server` test URI is used, and no `MONGO_URL` is required. This allows for out-of-the-box testing without a live database. | |
| 6. NODE_ENV=test --- When set to **"test"**, a `mongodb-memory-server` test URI is used, and no `MONGO_URL` is required. This allows for out-of-the-box testing without a live database. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.